Optimize file copies by using NIO methods #27459
Conversation
|
|
fmeum
force-pushed
the
nio-copy
branch
2 times, most recently
from
ce89326 to
6591ac3
Compare
github-actions
bot
added
the
awaiting-review
| throw translateNioToIoException(from.asFragment(), e); | ||
| } | ||
| return; | ||
| } |
There was a problem hiding this comment.
Can you also replace ByteStreams.copy(in, out) with in.transferTo(out) below? It can still do in-kernel I/O when both sides are files, and falls back to essentially equivalent code otherwise.
There was a problem hiding this comment.
I did that and realized that the copy fallback in moveFile can also be optimized in this way.
fmeum
changed the title
FileSystemUtils.copyFile via Files.copy if possible There was a problem hiding this comment.
Pull Request Overview
This PR refactors exception handling in the VFS implementation to centralize and standardize error message formatting across different FileSystem implementations. The key changes introduce a new translateNioToIoException utility method that maps NIO exceptions to consistent java.io exceptions with Unix-style error messages.
- Centralized exception translation logic in
FileSystem.translateNioToIoException() - Refactored
getIoFile()andgetNioPath()methods to returnnullinstead of throwingUnsupportedOperationException - Optimized file copy operations to use NIO fast paths when available
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| FileSystem.java | Added centralized translateNioToIoException() method and error message constants; changed getIoFile()/getNioPath() to return null instead of throwing exceptions |
| JavaIoFileSystem.java | Simplified exception handling in createSymbolicLink(), readSymbolicLink(), and renameTo() to use the new translation method; removed duplicate error constants |
| WindowsFileSystem.java | Updated createSymbolicLink() to use centralized exception translation |
| PathTransformingDelegateFileSystem.java | Added @Nullable annotations to getIoFile() and getNioPath() methods |
| FileSystemUtils.java | Added fast path for file copying using NIO APIs; replaced custom buffer copying with transferTo(); removed copyLargeBuffer() utility method |
| AbstractFileSystem.java | Added null-check wrapper for getIoFile() call; removed duplicate ERR_PERMISSION_DENIED constant |
Comments suppressed due to low confidence (1)
src/main/java/com/google/devtools/build/lib/vfs/AbstractFileSystem.java:99
- The
getNioPath(path)method now returns@Nullableas per the changes, but this code doesn't handle the null case. IfgetNioPath()returns null, this will throw aNullPointerExceptionwhen passed toFiles.newByteChannel(). A null check should be added to handle file systems that don't support NIO paths.
return Files.newByteChannel(getNioPath(path), READ_WRITE_BYTE_CHANNEL_OPEN_OPTIONS);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
`Files.copy` uses various optimizations such as kernel buffers (`sendfile` on Unix) or copy-on-write (`clonefile` on macOS, `copy_file_range` on Linux with a supported file system).
|
Importing this one myself. |
github-actions
bot
removed
the
awaiting-review
3 participants
Files.copyuses various optimizations such as kernel buffers (sendfileon Unix) or copy-on-write (clonefileon macOS,copy_file_rangeon Linux with a supported file system).InputStream.transferTocan use kernel buffers when both input streams areFileInputStreams.